Conversation
alexsohn1126
reviewed
Apr 9, 2026
Add the assistant:write scope to the Slack integration to enable the bot to act as a Slack Agent, supporting DM-based agent interfaces. Refs ISWF-2388 Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Extract shared org-resolution logic into _resolve_seer_organization helper and merge on_app_mention/on_dm into a single _handle_seer_mention method. Replace three identical halt reason enums with unified SeerSlackHaltReason. Extract duplicated loading messages list into a module-level constant. Refs ISWF-2388 Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
…nges Update tests to match the refactored _resolve_seer_organization which now iterates org integrations and uses SlackExplorerEntrypoint.has_access instead of checking a single feature flag. Align halt reasons with the consolidated enum values (NO_VALID_INTEGRATION, NO_VALID_ORGANIZATION) and update the has_access test for the seer-slack-explorer flag rename. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address PR review feedback: - Change SLACK_PROVIDERS from set to list to match the RPC method's expected `list[str]` parameter type, preventing serialization errors - Check org status before calling get_installation to avoid unnecessary queries for inactive orgs - Verify the requesting Slack user belongs to the resolved org when their identity is linked, preventing cross-org data access when multiple orgs share a Slack workspace - Fix inaccurate comments about DM fallback behavior Refs ISWF-2388 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5b77767 to
6f4a9d9
Compare
…uests Move identity verification to the start of org resolution so unlinked users receive an ephemeral prompt with a link button instead of silently failing downstream. Introduce SeerResolutionResult TypedDict to replace the tuple-or-None return pattern, add a static ephemeral message helper on SlackIntegration, and simplify DM dispatch by routing assistant-scoped integrations through the mention handler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Narrow handler signatures from SlackDMRequest to SlackEventRequest for better type safety. Extract channel_id, user_id, and thread_ts properties on SlackEventRequest to handle assistant_thread_started event structure differences in one place. Rename on_app_mention to on_prompt and is_assistant to has_assistant_scope for clarity. Add contextual messaging to the identity link prompt based on event type. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the ts fallback from SlackEventRequest.thread_ts so top-level messages correctly yield None instead of collapsing into the message ts. Add missing return type annotations to has_assistant_scope and is_assistant_thread_event. Move variable extraction before org resolution in _handle_seer_prompt for earlier lifecycle context.
Contributor
Backend Test FailuresFailures on
|
alexsohn1126
approved these changes
Apr 10, 2026
Member
alexsohn1126
left a comment
There was a problem hiding this comment.
Looks good, a couple of small things, should be good to push after!
tests/sentry/integrations/slack/webhooks/events/test_message_im.py
Outdated
Show resolved
Hide resolved
Move DM agent tests from test_message_im into a dedicated test_direct_message module. Simplify link_identity using create_identity helper and set up linked identities in setUp so individual tests only need to opt out. Revert thread_ts return type to str with or-None coercion at the call site.
Contributor
Backend Test FailuresFailures on
|
Extract ASSISTANT_THREAD as a typed dict[str, Any] to avoid mypy index errors from mixed-type dict value inference.
Convert LINK_SHARED_EVENT from JSON string to native dict and reuse BaseEventTest.link_identity and idp instead of manually creating identity providers and identities in each test.
The identity link prompt can be sent for top-level messages where thread_ts is None. Update the signature to accept str | None and reorder params to group slack_user_id with integration.
Add dict[str, Any] annotations to LINK_SHARED_EVENT and LINK_SHARED_EVENT_NO_CHANNEL_NAME to fix mixed-type inference. Remove leftover orjson.loads() calls and fix int-indexing on the now-dict event data.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec363ba. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

this allows DMs to the bot trigger Seer and gives a richer experience for starting conversations by suggesting prompts. to support this locally, you need to do a few things
assistant:writeOAuth scopeassistant_thread_startedAlso updated the linkage flow a bit, to better support dms
Refs ISWF-2388